add a cumsum transform to cumulatively sum a single column #7961

bryevdv · 2018-06-05T18:36:46Z

I ran across the SO question Create pie-chart using bokeh with dictionary
and thought I woudl see if it could be done with Stack but soon realized that it could not, since Stack stacks multiple entire columns element-wise. It occured to me that a cummulative sum was a very useful but missing piece that would also be trivial to implement, so I threw together this PR.

I will add docs and tests in subsequent commits.

bryevdv · 2018-06-05T18:37:33Z

With this PR the SO question can be achieved much more cleanly with:


from collections import Counter
from math import pi

import pandas as pd

from bokeh.palettes import Category20c
from bokeh.plotting import figure, show
from bokeh.transform import cumsum

x = Counter({
    'United States': 157,
    'United Kingdom': 93,
    'Japan': 89,
    'China': 63,
    'Germany': 44,
    'India': 42,
    'Italy': 40,
    'Australia': 35,
    'Brazil': 32,
    'France': 31,
    'Taiwan': 31,
    'Spain': 29
})

data = pd.DataFrame.from_dict(dict(x), orient='index').reset_index().rename(index=str, columns={0:'value', 'index':'country'})
data['angle'] = data['value']/sum(x.values()) * 2*pi
data['color'] = Category20c[len(x)]

p = figure(plot_height=350, title="Pie Chart", toolbar_location=None,
           tools="hover", tooltips=[("Country", "@country"),("Value", "@value")])

p.wedge(x=0, y=1, radius=0.4, 
        start_angle=cumsum('angle', include_zero=True), end_angle=cumsum('angle'),
        line_color="white", fill_color='color', legend='country', source=data)

p.axis.axis_label=None
p.axis.visible=False
p.grid.grid_line_color = None

show(p)

mattpap · 2018-06-05T20:05:12Z

bokehjs/src/lib/models/expressions/cumsum.ts

+        result[0] = 0
+    else
+        result[0] = col[0]
+    debugger


We should really just make a code quality check that looks for this

Sure, there's a built-in rule for this (https://palantir.github.io/tslint/rules/no-debugger/).

fixed in c591751

If your are at fixing things, you can add "no-debugger": true to bokehjs/tslint.json.

It's a part of codebase job on travis ci. Otherwise you have to run node make tslint. Perhaps we could make it part of quality tests at some point.

but it doesn't currently actually fail when there is a violation:

It was supposed to get --emit-error, the same as scripts:compile and test:compile have. Must have forgotten about this or maybe there is a regression since then new build is in place.

OK I added it, tho as I said it's not clear to me that a linter failure results in a test failure since the exit always seems to be 0

I tried adding --emit-error locally, that still did not result in a nonzero exit code when a lint violation was present

In fact this was done on purpose, because initially tslint reported a lot of errors and it was tedious dealing with this. Now all were resolved, so failing tslint build task on error should be implemented. I will do it with the next round of build improvements.

philippjfr · 2018-06-05T20:08:09Z

bokeh/models/expressions.py

+
+        source = ColumnDataSource(data=dict(foo=[1, 2, 3, 4]))
+
+        CumSum('foo')


Do Expressions actually allow positional arguments? Might be the case but maybe you confused it with the cumsum helper function signature?

No, this code is incorrect.

Also, it would be good to use actual Python shell syntax (>>>).

fixed in 7f2a0dc

mattpap · 2018-06-05T20:09:54Z

bokehjs/src/lib/models/expressions/cumsum.ts

+    if (this.include_zero)
+        result[0] = 0
+    else
+        result[0] = col[0]


This could have been reduced to one line as on the line before.

fixed in 0c3eaa9

add a cumsum transform to cumulatively sum a single column

7750660

bryevdv added the status: WIP label Jun 5, 2018

bryevdv added type: feature tag: component: bokehjs labels Jun 5, 2018

bryevdv added this to the 0.13.0 milestone Jun 5, 2018

add tests and docs

7d2d55f

bryevdv added status: ready and removed status: WIP labels Jun 5, 2018

bryevdv requested a review from mattpap June 5, 2018 19:25

mattpap reviewed Jun 5, 2018

View reviewed changes

philippjfr reviewed Jun 5, 2018

View reviewed changes

mattpap reviewed Jun 5, 2018

View reviewed changes

bryevdv added 4 commits June 5, 2018 13:10

remove errant debugger statement

c591751

fix docs mistake

7f2a0dc

simplify result initialization expression

0c3eaa9

add debugger check to linting

992b8c2

bryevdv merged commit 28a7f8c into master Jun 5, 2018

bryevdv deleted the bryanv/cumsum_transform branch June 5, 2018 21:36

bryevdv added status: accepted reso: completed and removed status: ready labels Jun 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add a cumsum transform to cumulatively sum a single column #7961

add a cumsum transform to cumulatively sum a single column #7961

bryevdv commented Jun 5, 2018

bryevdv commented Jun 5, 2018

mattpap Jun 5, 2018

bryevdv Jun 5, 2018

mattpap Jun 5, 2018

bryevdv Jun 5, 2018

mattpap Jun 5, 2018

mattpap Jun 5, 2018 •

edited

mattpap Jun 5, 2018

bryevdv Jun 5, 2018

bryevdv Jun 5, 2018

mattpap Jun 5, 2018

philippjfr Jun 5, 2018

mattpap Jun 5, 2018

mattpap Jun 5, 2018

bryevdv Jun 5, 2018

mattpap Jun 5, 2018

bryevdv Jun 5, 2018


		source = ColumnDataSource(data=dict(foo=[1, 2, 3, 4]))

		CumSum('foo')

add a cumsum transform to cumulatively sum a single column #7961

add a cumsum transform to cumulatively sum a single column #7961

Conversation

bryevdv commented Jun 5, 2018

bryevdv commented Jun 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattpap Jun 5, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattpap Jun 5, 2018 •

edited